[pull] master from facebook:master#530
Merged
pull[bot] merged 2 commits intoMLH-Fellowship:masterfrom Jan 17, 2021
Merged
Conversation
Summary: This implements the `MeasurableYogaNode` trait in `YogaLayoutableShadowNode`. We had this trait from the very beginning but never used it. Now, if the trait is specified, `YogaLayoutableShadowNode` will set the measure function for the node and dirty it during cloning. Previously, we used (and still use) a dedicated method for setting up the measure function - `YogaLayoutableShadowNode::enableMeasurement()`. The problem with it is that to make it work we have to dirty the Yoga node every time we clone it. And the only proper way to do this in the `YogaLayoutableShadowNode` constructor because if we do it later ancestor nodes could not observe this and react to this. Therefore we have to have a trait for it. The plan is to use it for TextInput first to fix a crash (see the next diff). After we confirm it works fine, we will replace all the usages of `enableMeasurement` with the new trait. This diff also renames the other two yoga-related traits (to make them less verbose and look unified), adds more comments, and asserts. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D25937711 fbshipit-source-id: fafbd5d62537ac09e02ffbfd56adab6d629d791d
Summary: It is the correct way to deal with the measure function. I believe it can fix the crash we see with TextInput state update. The crash is probably caused by invalid value of `shadowNodeRawPtr` in `YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector`. It's hard to reconstruct the full chain of events but I think it's related: Yoga's dirty flag influences cloning, so the improper setting of this flag can misalign "natural" ShadowNode cloning (influenced by changes in the tree) and YGNode-initiated cloning (triggered by layout process). Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D25937710 fbshipit-source-id: a4e7c9dd8f5e6598fc662f4c6ee8061fd795e20d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )